代码审查未发现新的 blocker/warning。

范围：移除 Soul Mode / CherryClaw 品牌和开关，把 autonomy tools、agent-memory、workspace prompt、channel/task eligibility 变为所有 agent 默认能力，并清理相关文档、i18n、settings UI 和测试。

影响：用户不再能按 agent 关闭原 Soul Mode 能力，旧 mcp__claw__* 命名/入口被移除。当前 GitHub merge state 仍是 DIRTY/有冲突，需要先同步 main 解决冲突后再重新确认最终 diff；因此这次先不 approve。

Review summary

Solid, well-tested PR. I found no cross-agent isolation break and no data-loss issue — cross-agent channel/task scoping is defense-in-depth and meaningfully tested, the i18n flattening is complete across all 12 locales, and the guard tests genuinely fail if a guard is removed.

The findings below are refinements, concentrated on one theme: now that every agent can run headless (scheduled / channel dispatch), a few "no interactive responder" stall gaps and an enlarged auto-approve blast radius deserve a second look. None are blockers; #1 is the most clearly actionable.

1. [Medium] Headless runs can still stall on plan-mode / worktree tools

src/main/ai/runtime/claudeCode/settingsBuilder.ts (disallowedTools, ~L835) · src/shared/ai/claudecode/constants.ts

The removed SOUL_MODE_DISALLOWED_TOOLS blocked 9 interactive tools. Only 3 (Cron* / TodoWrite / NotebookEdit) are re-covered by the registry's exposure:'disabled', and AskUserQuestion got a dedicated headless denial — but EnterPlanMode / ExitPlanMode / EnterWorktree (all exposure:'internal') have no headless guard. A scheduled/channel run that enters plan mode waits forever for an approval no one gives — exactly the stall the AskUserQuestion denial was added to prevent. Reads as an oversight rather than an intended widening.

Suggested fix: extend the isHeadless disallow branch to also block EnterPlanMode / ExitPlanMode (and consider EnterWorktree), mirroring the AskUserQuestion treatment.

2. [Medium] Interactive follow-up on a headless-baked warm connection silently loses AskUserQuestion

src/main/ai/runtime/claudeCode/settingsBuilder.ts (baked disallowedTools) + src/main/ai/agentSession/AgentSessionRuntimeService.ts (admitTurn / startNextTurn)

disallowedTools is baked into the SDK query once at connect time, keyed on the connection's headless flag. If an interactive UI message lands on a session while a headless turn is still running, the interactive follow-up rides the same headless-baked connection and never sees AskUserQuestion. The per-turn canUseTool gate can only add denials, not restore a baked-out tool. It self-heals once the session goes fully idle (the connectionHeadless !== headless mismatch rebuild), so the window is "interactive message arriving while a headless turn is busy, then back-to-back follow-ups."

Suggested fix: apply the same headless-mismatch teardown/rebuild at the busy-turn boundary, or drop the baked AskUserQuestion disallow and rely solely on the per-turn canUseTool gate (which already handles both directions symmetrically).

3. [Medium — by design, but broader now] Auto-approved config/cron/notify reachable by untrusted channel input on every agent

src/main/ai/runtime/claudeCode/settingsBuilder.ts (auto-approve prefix) · src/main/ai/mcp/servers/cherryAutonomyTools.ts

A third party who can message the agent over a connected IM channel can, via prompt injection, drive auto-approved config add_channel / remove_channel / rename, cron add, or notify (workspace file exfil to the attacker's chat) with no user approval. This stays correctly scoped to the messaged agent (no cross-agent break), but the PR broadens the surface from opt-in Soul agents to all agents, so the blast radius grew. The accepting comment near the auto-approve prefix acknowledges this.

Suggested mitigation: gate the mutating config actions (add_channel / remove_channel / rename) behind approval even when the read/notify/cron tools stay auto-approved, or deny them when the current turn is headless (mirroring the headless AskUserQuestion denial).

4. [Medium] Proxy-credential redaction — scheme-less edge leak + untested call-site wiring

src/main/ai/runtime/claudeCode/settingsBuilder.ts (redactProxyUrlForAssistantContext, ~L167 and its call site ~L213)

(a) app.proxy.url is free-text. For a scheme-less value like user:pass@host:8080, new URL() does not throw — it parses user: as the scheme, so username/password come back empty, the early return proxyUrl fires, and the raw credential lands in the assistant system prompt. (b) The function is unit-tested in isolation, but no test asserts the assembled context string is actually redacted — deleting the wrapper at the call site leaks credentials with every test still green.

Suggested fix: apply the userinfo-stripping regex unconditionally as a belt (not only in the catch branch), and add a buildClaudeCodeSessionSettings-level test asserting the produced context contains - Proxy: http://host and not the password.

5. [Low] Auto-approve is fail-open for future cherry-tools

src/main/ai/tools/adapters/claudeCode/cherryBuiltinApproval.ts · src/main/ai/tools/adapters/claudeCode/agentTools.ts

Auto-approval is prefix-based (mcp__cherry-tools__) minus a hardcoded kb_manage exception. Any tool later added to the cherry-tools server is auto-approved by default; a future mutating tool ships auto-approved unless the author remembers the exception, and no test would fail.

Suggested fix: invert to an explicit auto-approve allowlist, or add a test that every tool the cherry-tools server registers is classified into exactly one of the two lists. (Prefix shadowing is verified safe — trailing __, no standalone cherry server remains.)

6. [Low] Two one-line test gaps

• src/main/ai/runtime/claudeCode/__tests__/buildMcpServers.test.ts only positively checks cherry-tools/agent-memory are present; add expect(Object.keys(result)).not.toContain('skills') to lock in the "defined-but-unwired" invariant the docs describe.
• src/main/ai/runtime/claudeCode/__tests__/settingsBuilder.test.ts asserts the headless build's disallowedTools contains AskUserQuestion but not that notify survives; add not.toContain('mcp__cherry-tools__notify').

7. [Low — confirm intended] SKILLS_GUIDANCE prompt block dropped while the skills tool stays injected

src/main/ai/agents/prompt.ts

composeToolGuidance() no longer emits the skills usage guidance, but mcp__skills__skills remains in the registry (exposure:'internal') and its server is still built. The agent keeps the tool but loses all instructions on when/how to use it. Tangential to the "de-brand CherryClaw" scope — please confirm it's intentional.

Nits

• The config tool renders as "Agent Config" in the edit dialog (agent.tools.builtin.CherryConfig.label) but the hardcoded fallback in MCP_TOOL_LABELS is "Configuration" — same tool, two names.
• headlessMessageIds can retain stale ids on a turn-error path (bounded, cosmetic).
• The catch-branch redaction regex [^/@\s]+@ leaves a fragment on a malformed URL whose password contains an unencoded @ (extreme edge).

Strengths worth calling out

• Cross-agent isolation: every channel-id path validates channel.agentId === agentId with a non-existence-leaking "not found"; delivery re-validates at fire time; rebinds clear stale subscriptions (fail-safe on sync failure); the migrator skips cross-agent legacy rows — all with regression-catching tests (not.toHaveBeenCalled on the downstream mutation, plus a real file-backed integration test).
• Headless enforcement: cold / idle-reuse / in-flight-connecting rebuild paths are all correct and individually tested; resume token survives via DB re-hydration; rollHeadless steer semantics are exactly right. No stall gap for AskUserQuestion itself (only the plan-mode siblings in #1).
• i18n & docs: renamed keys present in all 12 locales including the dynamic notify/config tool-card keys; breaking-changes entry matches the final code; tool-registry/data-cluster docs correctly mark skills as unwired and notify/config as user-exposed; no broken links to the deleted cherryclaw/ docs.

Review performed read-only against the PR head; no code was changed.

This review was translated automatically.

Reviewed the previously blocking item: PR is no longer in DIRTY/conflict state, all GitHub checks for the current head have completed and passed. Approve.

Follow-up review — after fix(agent-runtime): tighten headless tool guards (64f19ff)

Thanks for the fast turnaround — this commit addresses my earlier review well:

• #1 headless plan-mode/worktree stall → fixed (HEADLESS_INTERACTIVE_TOOLS covers AskUserQuestion / EnterPlanMode / ExitPlanMode / EnterWorktree).
• #2 interactive follow-up losing AskUserQuestion on a headless-baked warm connection → fixed by dropping the baked headless disallow and enforcing per-turn in canUseTool.
• #3 untrusted-channel config mutation → hardened with headlessConfigMutationHook.
• #4 proxy-credential redaction → scheme-less case + assembled-context wiring both covered.
• #5 auto-approve fail-open prefix → switched to an explicit allowlist with a regression test.
• #6 the two missing test assertions → added.

One new gap the refactor introduces, plus one cleanup:

[Medium] Headless interactive-tool denial is skipped under bypassPermissions / acceptEdits

src/main/ai/runtime/claudeCode/settingsBuilder.ts

This commit moved the interactive-tool denial out of disallowedTools and into canUseTool (the per-turn gate). But the SDK skips canUseTool for auto-approved paths — this file's own comment (~L799) states it: "the SDK skips canUseTool for auto-approved paths (bypassPermissions / acceptEdits / default safe-tools), but PreToolUse hooks fire on every tool call regardless of permission mode." That is exactly why the sibling config-mutation guard in this same commit is a PreToolUse hook rather than a canUseTool branch.

Since permissionMode: agentConfig?.permission_mode (L333) passes the user-set mode through unmodified, a scheduled/channel (headless) agent can run in bypassPermissions — which is in fact the mode the old Soul Mode used to force, so migrated autonomy agents plausibly carry it. In that case, for a non-assistant agent, all three enforcement layers miss:

• disallowedTools — no longer lists these tools for non-assistant agents (the isHeadless branch was removed here),
• canUseTool — skipped under bypass/acceptEdits,
• PreToolUse hooks — none registered for the interactive tools (only for config mutation).

Net: AskUserQuestion / EnterPlanMode / ExitPlanMode / EnterWorktree become reachable again in a headless bypass run → the exact stall #1 set out to prevent. This is also a partial regression from the pre-commit state, where the isHeadless branch hard-baked AskUserQuestion into disallowedTools (which does apply under bypass).

Suggested fix: enforce the interactive-tool denial via a PreToolUse hook too (mirroring headlessConfigMutationHook, which fires regardless of permission mode), keeping the canUseTool branch as well so interactive follow-ups can still recover. That makes the two headless guards symmetric and closes the bypass hole without reopening #2.

Worth confirming: whether AskUserQuestion counts as a "default safe-tool" that also skips canUseTool in default mode — if so, the gap is wider than just bypass/acceptEdits.

[Low / cleanup] headless option and the connection-rebuild machinery may now be dead

buildClaudeCodeSessionSettings still accepts headless?: boolean (settingsBuilder.ts:258) but nothing in the function reads it anymore — the baked settings no longer vary by headless. The connectionHeadless mismatch teardown/rebuild in AgentSessionRuntimeService was added specifically because "the AskUserQuestion disallow is settable only at connect time"; that rationale is gone now that enforcement is per-turn. Worth confirming and removing the now-redundant option + rebuild path (and its tests) to avoid leaving dead complexity.

Review performed read-only against the PR head; no code was changed.

LGTM — approving.

All review items across the three rounds are resolved:

• Initial review (#1 headless plan-mode/worktree stall, #2 warm-connection AskUserQuestion loss, #3 auto-approved config-mutation surface, #4 proxy-credential redaction, #5 fail-open auto-approve prefix, #6 test-assertion gaps) — all fixed.
• Follow-up (interactive-tool denial skipped under bypassPermissions/acceptEdits, plus the now-dead headless connection plumbing) — fixed in 5d0b762: the denial is now a PreToolUse hook (fires in every permission mode) beside the retained canUseTool branch, both resolved per-turn by session id via isCurrentTurnHeadless; the redundant connect-time headless plumbing and mismatch-rebuild were cleanly removed with per-turn enforcement preserved and tests updated.

CI is green (basic-checks, catalog-hand-edit-check, changes, changeset-check, general-test, render-test).

One non-blocking confirmation (not required for merge): the SKILLS_GUIDANCE prompt block was dropped while the mcp__skills__skills tool stays injected — please confirm that leaving the tool available without usage guidance is intentional. Not a merge blocker.

Nice work on the cross-agent scoping and the headless-enforcement hardening.

Requesting changes for five confirmed issues:

1. [Blocking] settingsBuilder.ts:82 and cherryAutonomyTools.ts:769 leave bootstrap config writes available in headless turns. HEADLESS_CONFIG_MUTATION_ACTIONS denies rename and channel mutations, but complete_bootstrap and reset_bootstrap also call agentService.updateAgent() to mutate agent.configuration.bootstrap_completed. Since bootstrap instructions explicitly ask the model to call config complete_bootstrap, a scheduled or channel run can still mutate agent configuration even though the headless hook says these turns cannot mutate configuration. Please add both bootstrap actions to the headless deny set, or document them as intentional exceptions and adjust the policy/tests.

2. [Blocking] AgentChannelWorkflowService.ts:72 loses existing task subscriptions when a channel rebind fails. The code clears subscriptions when agentId changes, then if syncChannel() throws it restores the channel row to the old agent and clears subscriptions again. That leaves a failed update with the original channel owner restored but all pre-existing agent_channel_task rows deleted. Please snapshot and restore the old subscriptions on rollback, or make the rebind and subscription cleanup rollbackable as one operation.

3. [Blocking] AgentTaskService.ts:202 can partially update a task when subscription replacement fails. updateJobSchedule() commits the schedule/name/prompt/trigger changes before replaceTaskSubscriptions() runs. If subscription replacement throws, the API returns an error but the task fields remain updated while channels remain old. createTask already rolls back the registered schedule on this failure path, but updateTask has no equivalent rollback. Please wrap the schedule update and subscription replacement in a single atomic write, or explicitly restore the previous schedule when the subscription write fails.

4. [Important] TasksSettings.tsx:328 keeps hidden stale channelIds in the task detail state. The UI filters the selector options to channels owned by the task agent, but initializes and resyncs channelIds from task.channelIds without filtering. The shared Combobox keeps unknown selected values in its controlled array, so when the user changes visible channel selections the PATCH can still include a hidden foreign or missing id. The service now rejects that id, which means the user cannot clear the stale value from this UI. Please normalize channelIds against the visible/owned taskChannels on load and before save.

5. [Important] zh-cn.json:41 / zh-tw.json:41 and zh-cn.json:661 / zh-tw.json:661 contain empty user-visible error strings. agent.channels.createError, deleteError, updateError, agent.tasks.error.loadFailed, and agent.tasks.logs.loadError are blank in both Chinese locale files while English has real text. The affected toasts and empty states render as blank messages in Chinese. Please fill these migrated locale keys with non-empty localized text.